-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
riscv64: Add support for load+extend
patterns
#8765
Conversation
RISC-V doesen't have sinkable loads per se, but the regular load instructions support sign / zero extending the loaded values. We model those here as a sinkable load on the extend instruction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice!
;; Extract a sinkable instruction from a value operand. | ||
(decl sinkable_inst (Inst) Value) | ||
(extern extractor sinkable_inst sinkable_inst) | ||
|
||
;; Matches a sinkable load. | ||
(decl sinkable_load (Inst Type MemFlags Value Offset32) Value) | ||
(extractor (sinkable_load inst ty flags addr offset) | ||
(and | ||
(load flags addr offset) | ||
(sinkable_inst (has_type ty inst)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a comment here about the extend(load())
use case and the asterisk around "sinkable" that you added in the PR description? That seems like good context to have when reading this code.
Subscribe to Label Action
This issue or pull request has been labeled: "cranelift", "cranelift:area:riscv64", "isle"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
This commit adds support for merging a load with a `{u,s}extend` instruction. On AArch64 the load instructions already do this by default, so we can just emit the regular loads. See also bytecodealliance#8765 that does a similar thing for RISC-V
This commit adds support for merging a load with a `{u,s}extend` instruction. On AArch64 the load instructions already do this by default, so we can just emit the regular loads. See also bytecodealliance#8765 that does a similar thing for RISC-V
This commit adds support for merging a load with a `{u,s}extend` instruction. On AArch64 the load instructions already do this by default, so we can just emit the regular loads. See also #8765 that does a similar thing for RISC-V
👋 Hey,
This PR adds support for merging
{s,u}extend
instructions into a precedingload
.RISC-V doesn't have sinkable loads per se, but the regular load instructions sign / zero extend the loaded values by default. So here we model that by pretending that that is a sinkable load on an extend instruction.
This PR is also a part of #6056. I'm working on that, the first step is to support generating the same code with
load+extend
on all backends as we currently do with the specialized{u,s}loadNN
instructions.